Skip to content

feat: update types to allow type narrowing#535

Merged
vcapretz merged 2 commits intocanaryfrom
improve-type-narrowing
Jun 13, 2025
Merged

feat: update types to allow type narrowing#535
vcapretz merged 2 commits intocanaryfrom
improve-type-narrowing

Conversation

@vcapretz
Copy link
Copy Markdown
Contributor

closes #356

previously we had the response schemas defining both data and error to be either the value or null. but as correctly pointed in the issue, we always have either/or

by updating the type definitions to something like:

// from this:
type Response = { data: MyData | null; error: MyError | null }

// to this:
type Response = { data: MyData; error: null } | { data: null; error: MyError }

we can leverage TypeScript type narrowing in the code and have better types 🎉

note the null in here:
CleanShot 2025-06-13 at 16 42 14@2x

much better:
CleanShot 2025-06-13 at 16 51 10@2x

@vcapretz vcapretz self-assigned this Jun 13, 2025
@vcapretz vcapretz requested a review from a team as a code owner June 13, 2025 19:55
@vcapretz vcapretz merged commit e327235 into canary Jun 13, 2025
9 checks passed
@vcapretz vcapretz deleted the improve-type-narrowing branch June 13, 2025 20:16
@tadeumaia
Copy link
Copy Markdown

tadeumaia commented Jul 22, 2025

Should changing types be a major bump on semver as it causes breaking code?

Works perfectly fine on 4.6 but breaks on 4.7

  it("should throw error if Resend API fails", async () => {
    // Arrange
    const mockResendResponse: ResendEmailResponse = {
      data: null,
      error: {
        message: "Failed to send email",
        name: "invalid_parameter",
      },
    };
    vi.mocked(resend.emails.send).mockResolvedValue(mockResendResponse);

    // Act & Assert
    await expect(sendInvitationEmail(mockEmailData)).rejects.toThrow("Failed to send invitation email");
  });
tests/unit/lib/email/index.test.ts:499:53 - error TS2345: Argument of type 'ResendEmailResponse' is not assignable to parameter of type 'CreateEmailResponse'.
  Type 'ResendEmailResponse' is not assignable to type '{ data: null; error: ErrorResponse; }'.
    Types of property 'data' are incompatible.
      Type '{ id: string; } | null' is not assignable to type 'null'.
        Type '{ id: string; }' is not assignable to type 'null'.

499     vi.mocked(resend.emails.send).mockResolvedValue(mockResendResponse);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Type annotations improvement

3 participants